azure AD identity support for AzureAD-enabled AKS clusters #205

erhancagirici · 2023-12-05T12:40:54Z

Description of your changes

Enables configuring Azure AD authentication via kubelogin integration. Introduces the type AzurePrincipalCredentials in ProviderConfig .spec.identity section.

The specified Azure Service Principal credentials provided via Secret, for authenticating to AKS cluster by obtaining a token through kubelogin

An example ProviderConfig using AzurePrincipalCredentials as identity for authenticating to AzureAD

apiVersion: helm.crossplane.io/v1beta1
kind: ProviderConfig
metadata:
  name: helm-provider
spec:
  credentials:
    source: Secret
    secretRef:
      name: cluster-config
      namespace: crossplane-system
      key: kubeconfig
  identity:
    type: AzurePrincipalCredentials
    source: Secret
    secretRef:
      name: azure-credentials
      namespace: crossplane-system
      key: credentials.json

Fixes #180

I have:

Read and followed Crossplane's contribution process.
Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Tested with 2 different AKS clusters.

Azure AD auth enabled with Kubernetes RBAC
Azure AD auth enabled with Azure RBAC

The provider configs consist of the following:

For credentials: the kubeconfig AKS cluster, obtained via az aks get-credentials --resource-group myresourcegroup --name myclustername, as described in the Azure Portal > AKS cluster > Connect section
For identity: an azure service principal

For cluster 1 (AzureAD Auth + k8s RBAC): The service principal is added to a Azure AD group, then that group is added to Cluster admin ClusterRoleBinding at Azure Portal > The AKS cluster > Cluster Configuration

For cluster 2 (AzureAD Auth + Azure RBAC): The service principal is assigned Azure Kubernetes Service RBAC Cluster Admin built-in role for the particular AKS cluster, through Azure Portal > The AKS cluster > Access Control (IAM) > Add Role Assignment. If desired, a custom role can be built that has less privileges and assigned to the service principal.

Using those ProviderConfigs, an example Helm Release manifest is created and reconciled successfully.

Signed-off-by: Erhan Cagirici <erhan@upbound.io>

haarchri · 2023-12-05T18:16:33Z

what happen if you use the kubeconfig from connectionSecret ?

erhancagirici · 2023-12-05T18:40:33Z

what happen if you use the kubeconfig from connectionSecret ?

@haarchri summarizing our slack chat for reference here:
it does not make a difference, since this PR is irrelevant of how you supply the the kubeconfig, and does not change anything regarding that.
The content of the kubeconfig is important here, which is produced based on the relevant AKS cluster configuration:

If the AKS cluster was configured as Azure AD auth with k8s RBAC or Azure AD Auth with Azure RBAC the produced kubeconfig has the content that require kubelogin, therefore the new identity section in ProviderConfig
If configured with Local accounts with k8s RBAC the produced kubeconfig can work standalone, without any helper tool. In this case, you won't need the identity section in ProviderConfig

Signed-off-by: Erhan Cagirici <erhan@upbound.io>

haarchri · 2023-12-06T18:22:08Z

tested is working:

kubectl get releases
NAME        CHART   VERSION  SYNCED  READY  STATE   REVISION  DESCRIPTION    AGE
test-aks-7djh7  argo-cd  5.51.1  True   True  deployed  1     Install complete  19m

haarchri · 2023-12-06T20:19:06Z

one side note:

kubectl logs -n upbound-system       crossplane-contrib-provider-helm-877f6679cb2f-84c54c7747-r2qsg 
[controller-runtime] log.SetLogger(...) was never called; logs will not be displayed.
Detected at:
	>  goroutine 161 [running]:
	>  runtime/debug.Stack()
	>  	runtime/debug/stack.go:24 +0x64
	>  sigs.k8s.io/controller-runtime/pkg/log.eventuallyFulfillRoot()
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/log/log.go:60 +0xf4
	>  sigs.k8s.io/controller-runtime/pkg/log.(*delegatingLogSink).WithValues(0x4000059640, {0x40006021c0, 0x2, 0x2})
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/log/deleg.go:168 +0x44
	>  github.com/go-logr/logr.Logger.WithValues(...)
	>  	github.com/go-logr/logr@v1.3.0/logr.go:336
	>  sigs.k8s.io/controller-runtime/pkg/builder.(*Builder).doController.func1(0x40006021a0)
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/builder/controller.go:400 +0x130
	>  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0x4000686fa0, {0x1f4bc60, 0x4000681720}, {0x19d4120?, 0x40006020c0?})
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:306 +0x114
	>  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0x4000686fa0, {0x1f4bc60, 0x4000681720})
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:266 +0x198
	>  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:227 +0x74
	>  created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 in goroutine 56
	>  	sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:223 +0x43c

turkenh · 2023-12-06T20:34:49Z

one side note:

Same as crossplane-contrib/provider-upjet-aws#854 (comment)

…essage Signed-off-by: Erhan Cagirici <erhan@upbound.io>

turkenh

Great work @erhancagirici 💪

Left a couple of comments but nothing major.

turkenh · 2023-12-07T06:53:49Z

apis/v1beta1/types.go

@@ -50,12 +50,14 @@ type IdentityType string
 // Supported identity types.
 const (
 	IdentityTypeGoogleApplicationCredentials = "GoogleApplicationCredentials"
+
+	IdentityTypeAzurePrincipalCredentials = "AzurePrincipalCredentials"


Suggested change

IdentityTypeAzurePrincipalCredentials = "AzurePrincipalCredentials"

IdentityTypeAzurePrincipalCredentials = "AzureServicePrincipalCredentials"

Not sure if there is something called as AzurePrincipal vs AzureServicePrincipal but more explicit would be better in the API.

Principal is a generic term for referring to an authentication subject. I named it like this in case we support other principals like MSI or workload identity and not directly suggest Service Principals. I am OK with changing to AzureServicePrincipalCredentials or some other name

I would suggest so considering we only support ServicePrincipalLogin and to be explicit there.

cmd/provider/main.go

pkg/clients/azure/azure.go

pkg/controller/release/release.go

Signed-off-by: Hasan Turken <turkenh@gmail.com>

…eject injected identity source - update comments for opt parsing, cleanup comments Signed-off-by: Erhan Cagirici <erhan@upbound.io>

Signed-off-by: Erhan Cagirici <erhan@upbound.io>

turkenh

Thank you @erhancagirici 🙌

erhancagirici changed the title ~~azure AD identity support for AzureAD clusters~~ azure AD identity support for AzureAD-enabled AKS clusters Dec 5, 2023

turkenh self-requested a review December 5, 2023 12:59

erhancagirici force-pushed the azure-kubelogin-support branch 3 times, most recently from fcf92bd to f5aa5e1 Compare December 5, 2023 15:11

azure AD identity support for AzureAD clusters

8db230f

Signed-off-by: Erhan Cagirici <erhan@upbound.io>

erhancagirici force-pushed the azure-kubelogin-support branch from f5aa5e1 to 8db230f Compare December 5, 2023 15:21

update build submodule

e7c2c1e

Signed-off-by: Erhan Cagirici <erhan@upbound.io>

turkenh mentioned this pull request Dec 6, 2023

export newTokenProvider function for library usage Azure/kubelogin#371

Closed

explicitly set no-op logger for controller-runtime to avoid warning m…

3ace72a

…essage Signed-off-by: Erhan Cagirici <erhan@upbound.io>

erhancagirici force-pushed the azure-kubelogin-support branch from e0b38df to 3ace72a Compare December 6, 2023 23:04

turkenh reviewed Dec 7, 2023

View reviewed changes

Fix linter and unit tests

17ceb75

Signed-off-by: Hasan Turken <turkenh@gmail.com>

turkenh force-pushed the azure-kubelogin-support branch from f9c60fc to 17ceb75 Compare December 7, 2023 07:12

refactor identity type name to AzureServicePrincipalCredentials and r…

1ec4c08

…eject injected identity source - update comments for opt parsing, cleanup comments Signed-off-by: Erhan Cagirici <erhan@upbound.io>

erhancagirici requested a review from turkenh December 7, 2023 09:03

add unit tests for azure identity section

dc12e5a

Signed-off-by: Erhan Cagirici <erhan@upbound.io>

turkenh approved these changes Dec 8, 2023

View reviewed changes

erhancagirici merged commit 6ba133b into crossplane-contrib:master Dec 8, 2023
8 checks passed

erhancagirici deleted the azure-kubelogin-support branch December 14, 2023 12:33

erhancagirici mentioned this pull request Dec 14, 2023

identity section for AzureAD auth in ProviderConfig should not be processed when the supplied kubeconfig does not require AzureAD auth #206

Open

This was referenced Dec 15, 2023

Integrate kubelogin for Azure AD Authentication to AKS Clusters crossplane-contrib/provider-kubernetes#105

Closed

feat(kubelogin): add kubelogin azure ad auth crossplane-contrib/provider-kubernetes#170

Merged

bcho mentioned this pull request Jan 3, 2024

feat: import kubelogin library mode #207

Open

2 tasks

turkenh mentioned this pull request Jan 8, 2024

[WIP] Kubelogin Integration #203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azure AD identity support for AzureAD-enabled AKS clusters #205

azure AD identity support for AzureAD-enabled AKS clusters #205

erhancagirici commented Dec 5, 2023

haarchri commented Dec 5, 2023

erhancagirici commented Dec 5, 2023 •

edited

Loading

haarchri commented Dec 6, 2023

haarchri commented Dec 6, 2023 •

edited

Loading

turkenh commented Dec 6, 2023 •

edited

Loading

turkenh left a comment

turkenh Dec 7, 2023

erhancagirici Dec 7, 2023

turkenh Dec 8, 2023

turkenh left a comment

	IdentityTypeAzurePrincipalCredentials = "AzurePrincipalCredentials"
	IdentityTypeAzurePrincipalCredentials = "AzureServicePrincipalCredentials"

azure AD identity support for AzureAD-enabled AKS clusters #205

azure AD identity support for AzureAD-enabled AKS clusters #205

Conversation

erhancagirici commented Dec 5, 2023

Description of your changes

How has this code been tested

haarchri commented Dec 5, 2023

erhancagirici commented Dec 5, 2023 • edited Loading

haarchri commented Dec 6, 2023

haarchri commented Dec 6, 2023 • edited Loading

turkenh commented Dec 6, 2023 • edited Loading

turkenh left a comment

Choose a reason for hiding this comment

turkenh Dec 7, 2023

Choose a reason for hiding this comment

erhancagirici Dec 7, 2023

Choose a reason for hiding this comment

turkenh Dec 8, 2023

Choose a reason for hiding this comment

turkenh left a comment

Choose a reason for hiding this comment

erhancagirici commented Dec 5, 2023 •

edited

Loading

haarchri commented Dec 6, 2023 •

edited

Loading

turkenh commented Dec 6, 2023 •

edited

Loading